Skip to content

Conversation

@Jinchul81
Copy link

Warnings have been suppressed according to the following patterns:

  • Applied [[maybe_unused]]
  • Explicitly initialize the member variables in struct.
  • Unaligned type comparison between signed and unsigned. Converted unsigned to singed.
  • Hiding virtual functions in the base class. Declare using base::func.

Copy link
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I've left a minor comment. Please also fix the linter errors.

std::unordered_map<std::string, std::string> defaults; // required
std::unordered_map<std::string, std::string> overrides; // required
std::vector<std::string> endpoints;
std::unordered_map<std::string, std::string> defaults{}; // required
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to add {} for everything, at least not for STL containers and std::string. Please try to minimize lines of change like this.

const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec,
const std::string& location,
const std::unordered_map<std::string, std::string>& properties) {
[[maybe_unused]] const TableIdentifier& identifier,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove the parameters' names instead of adding [[maybe_unused]] to keep the code clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants